Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Integrate d2-logger (coding dojo ----DO NOT MERGE----- ) #23

Open
wants to merge 13 commits into
base: development
Choose a base branch
from

Conversation

deeonwuli
Copy link
Contributor

@deeonwuli deeonwuli commented Sep 25, 2024

📌 References

📝 Implementation

  • Integrating d2-logger to mapping script

📹 Screenshots/Screen capture

🔥 Notes to the tester

To test:
yarn script-map-outbreak-to-alerts
Logs are saved as events in the RTSL_ZEBRA_MAPPING_LOGGER event program [usU7YBzuhaE].

#8695pt0av

@deeonwuli deeonwuli changed the title Feat/d2 logger [feature] Integrate d2-logger to mapping script Sep 25, 2024
@deeonwuli deeonwuli changed the title [feature] Integrate d2-logger to mapping script [feature] Integrate d2-logger (coding dojo, do not merge) Oct 15, 2024
@bhavananarayanan bhavananarayanan changed the title [feature] Integrate d2-logger (coding dojo, do not merge) [feature] Integrate d2-logger (coding dojo, DO NOT MERGE ) Oct 15, 2024
@bhavananarayanan bhavananarayanan changed the title [feature] Integrate d2-logger (coding dojo, DO NOT MERGE ) [feature] Integrate d2-logger (coding dojo ----DO NOT MERGE----- ) Oct 15, 2024
@deeonwuli deeonwuli marked this pull request as ready for review October 18, 2024 16:09
Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Just some details and we're done

const mapping: Record<OutbreakDataType, TrackedEntityAttributeId> = {
disease: RTSL_ZEBRA_ALERTS_DISEASE_TEA_ID,
hazard: RTSL_ZEBRA_ALERTS_EVENT_TYPE_TEA_ID,
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the mapping is static, it can be defined at the root level and the function.

src/data/repositories/AlertD2Repository.ts Show resolved Hide resolved
}[];
} & {
[key in OutbreakDataType]?: string;
};
Copy link

@tokland tokland Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this a little bit on the Dojo, what was the conclusion? I I think this should work:

{
   //...
    type: OutbreakDataType;
    description: string // "description" or other name
    //...
}

This works if the type only has one of the types, but that's the case, right?

const mapping: Record<OutbreakDataType, DataSource> = {
disease: DataSource.RTSL_ZEB_OS_DATA_SOURCE_IBS,
hazard: DataSource.RTSL_ZEB_OS_DATA_SOURCE_EBS,
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same.

throw new Error("VITE_DHIS2_AUTH must be set in the .env file");

const username = process.env.VITE_DHIS2_AUTH.split(":")[0] ?? "";
const password = process.env.VITE_DHIS2_AUTH.split(":")[1] ?? "";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal:

const [username, password] = process.env.VITE_DHIS2_AUTH.split(":")

if (!username || !password)

Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good code-wise!

import {
RTSL_ZEBRA_ALERTS_DISEASE_TEA_ID,
RTSL_ZEBRA_ALERTS_EVENT_TYPE_TEA_ID,
} from "../consts/DiseaseOutbreakConstants";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change, but to avoid long imports and dispersion of info, values can be grouped in the constants file by some criteria.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants